Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Paged keys rpc for child storage. #9100

Merged
6 commits merged into from
Jul 7, 2021
Merged

Conversation

cheme
Copy link
Contributor

@cheme cheme commented Jun 14, 2021

Paged rpc childstate_getKeysPaged for child storage on the same model as state_getKeysPaged.

closes #9086.

polkadot companion: paritytech/polkadot#3258

@cheme cheme added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jun 14, 2021
@cheme cheme added the D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. label Jun 14, 2021
@shawntabrizi
Copy link
Member

shawntabrizi commented Jun 14, 2021

thank you! cc @jacogr

@@ -34,6 +34,7 @@ pub trait ChildStateApi<Hash> {
/// RPC Metadata
type Metadata;

/// DEPRECATED: Please use `childstate_getKeysPaged` with proper paging support.
Copy link
Member

@shawntabrizi shawntabrizi Jun 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not actually mark this as deprecated somewhere?

What is the process to eventually get rid of this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not really sure. This is just text that will show in rust doc, I copy pasted it from its top storage counterpart.
Maybe there is a better way or could be interesting to remove in a next version with other impacting rpc changes
(I thing the comment for the state function was here for rather long).

Copy link
Contributor

@apopiak apopiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some doc improvements

client/api/src/backend.rs Outdated Show resolved Hide resolved
client/api/src/backend.rs Outdated Show resolved Hide resolved
cheme and others added 2 commits June 16, 2021 11:45
Co-authored-by: Alexander Popiak <alexander.popiak@gmail.com>
Co-authored-by: Alexander Popiak <alexander.popiak@gmail.com>
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Dunno about the deprecated thing though. Feels we should clean it up

@cheme
Copy link
Contributor Author

cheme commented Jul 7, 2021

I like the fact that this PR does not break anything, I think I will open a different one removing the deprecated api.

@cheme
Copy link
Contributor Author

cheme commented Jul 7, 2021

bot merge

@ghost
Copy link

ghost commented Jul 7, 2021

Trying merge.

@ghost ghost merged commit 7a3a090 into paritytech:master Jul 7, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

childstate_getKeysPaged
5 participants